Skip to content

Migrate CI build-doc to Meson #39973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Apr 19, 2025

Make it possible to build the documentation with meson via

meson compile -C builddir doc-html

For this a couple of changes was necessary in the docbuilder:

  • Make sage_docbuild independent of sage so that meson can use sage_docbuild during config time to construct all the docbuild targets (otherwise one needs to first install sage, and then could configure the docbuild)
  • Properly handle input and output dirs for the docbuild, without relying on magic sage env variables

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@user202729
Copy link
Contributor

Looks like some file permission goes wrong? (symbolic link get converted to normal file?)

docs = [doc[1] for doc in sorted(documents)]
# Put the bibliography first, because it needs to be built first:
docs.remove(Path('reference/references'))
docs.insert(0, Path('reference/references'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is such a large change necessary in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I got carried a way a bit: the strictly necessary change is to remove mentions of import sage, then I used the chance to use pathlib, and to catch all those paths vs strings issues added typing info. Oh, and there were a few old deprecated stuff in there which was easier to remove than to migrate to pathlib 🦊

Copy link

github-actions bot commented May 17, 2025

Documentation preview for this PR (built with commit 1c65a0b; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@tobiasdiez tobiasdiez marked this pull request as ready for review May 17, 2025 10:23
@tobiasdiez
Copy link
Contributor Author

Doc build is working now, and the changes relative to the last develop build seem to be minimal and mostly due to using a newer sphinx version now.

Marking it as blocker since currently the docbuild CI for other PRs is completely failing.

@dimpase
Copy link
Member

dimpase commented May 17, 2025

https://github.com/sagemath/sage/actions/runs/15081532937/job/42398821050#step:9:1

fails while building docs the old way - is it normal?

@tobiasdiez
Copy link
Contributor Author

It fails with

ImportError: libSingular-4.4.0.so: cannot open shared object file: No such file or directory

https://github.com/sagemath/sage/actions/runs/15081532937/job/42398821050#step:9:3982 so this is the same problem as for the other PRs.

@dimpase
Copy link
Member

dimpase commented May 17, 2025

no, my question was - why does that CI run build docs the old way?

@tobiasdiez
Copy link
Contributor Author

But because it's now marked as changed, the doctest gets explicitly invoked on it.

Hm, the doctest framework has a mechanism to mark something as not tested. (See https://github.com/sagemath/sage/pull/39102/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721R123 ). Maybe it is also applicable here?

Thanks, I was not aware of this mechanism!

I think it's the same issue as #39467 again.
either it is supported, then the code to test it should remain in the code base

I think the situation here is a bit different. The doc build ci workflow is mostly there to provide devs a preview of the doc changes; it's purpose is less to test the docbuilding itself (of course it's testing this as well). Also the meson and make scripts just invoke the sage docbuilder, which is doing most of the work - so I don't think it's worth to test both ways.

But yeah, it's a complicated question on how to best migrate completely to meson, also with respect to the required doc changes. On one hand, it's a lot of additional work to maintain the two systems in parallel (including docs and ci for both) while on the other hand you would want to make the transition as smooth as possible. I'm hoping that we can soft-deprecate the make interface in the next release cycle, which would be then a natural point to revise and migrate more of the documentation to meson. I'm open and happy about other opinions on how this migration should look like.

@user202729
Copy link
Contributor

Thanks, I was not aware of this mechanism!

do you plan to fix this later or just ignore it for this pull request?

@tobiasdiez
Copy link
Contributor Author

Thanks, I was not aware of this mechanism!

do you plan to fix this later or just ignore it for this pull request?

Just wanted to add it to this PR, only to realize that it's not going to be that easy: the new workflow is just using ordinary git commands to stage all changed files (relative to develop), and then calls sage test --new. Probably can be fixed by committing changes to certain directories again manually; but that's definitely something for another PR.

@dimpase
Copy link
Member

dimpase commented May 19, 2025

@user202729 - are you happy enough here now to revert this to positive review?

@user202729
Copy link
Contributor

user202729 commented May 19, 2025

Actually:

  • maybe change/append the PR title to "Migrate CI build-doc to meson"?
  • currently build-doc-pdf is not tested at all? What's the plan for that?

@user202729
Copy link
Contributor

Also the meson and make scripts just invoke the sage docbuilder, which is doing most of the work - so I don't think it's worth to test both ways.

Actually, the test is exactly doing its jobs, no?

In other words: is #40120 just an issue with the CI, or does an actual user using sage-the-distro (since that isn't completely deprecated yet we ought to assume some user exists…) trying to build doc that way would encounter the same error? The latter appears more likely? (I'm not using sage-the-distro myself)

In that case if the CI infrastructure is removed how do we know if a similar breakage won't happen again with some other package upgrade?

@tobiasdiez tobiasdiez changed the title Meson: add building of documentation as target Migrate CI build-doc to Meson May 20, 2025
@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented May 20, 2025

Actually:

* maybe change/append the PR title to "Migrate CI build-doc to meson"?

Done.

* currently build-doc-pdf is not tested at all? What's the plan for that?

I don't have a plan for this atm. To be honest, I don't even see the point of building a pdf of the docs in 2025. Having a good online documentation should suffice.

In other words: is #40120 just an issue with the CI, or does an actual user using sage-the-distro (since that isn't completely deprecated yet we ought to assume some user exists…) trying to build doc that way would encounter the same error?

The error is actually not related to the docs build. What happened is that the cython module was compiled with Singular x. Then Singular was updated to version y. The problem is that the CI/sage-the-distro doesn't recognize that the version of Singular changed and the cython module needs to be compiled again to link against Singular y and not x. So the underlying issue is with the caching of the CI/sage-the-distro.

@user202729
Copy link
Contributor

So the underlying issue is with the caching of the CI/sage-the-distro.

Indeed, I also guessed that. But isn't the CI caching the exact same mechanism as the normal caching anyway (e.g. an user of sage-the-distro originally use 4.4.0, then git pull to upgrade it to 4.4.1, will face the same error)

Anyway… can we somehow delete the CI cache to fix test-long for now?

I don't even see the point of building a pdf of the docs in 2025

In some rare cases, this catches some LaTeX errors… as a fallback if the author forget to check the documentation diff.

(Also, some HTML pages are extremely large that it lags my browser, it might be beneficial for someone to look at the PDF, because it's paginated it should render at reasonable speed)

@user202729
Copy link
Contributor

@vbraun What's the issue?

@tobiasdiez
Copy link
Contributor Author

@vbraun is it because of http://build.sagemath.org/#/builders/47/builds/124? This indeed fails for the docs, but the full error points more to the m4ri update as the source.

@vbraun
Copy link
Member

vbraun commented May 28, 2025

Does pdf docbuild work? I don't remember what the error was, but pretty sure the merge script failed because of that.

And yes, the point of running TeX is to verify that the equation are valid markup.

@dimpase
Copy link
Member

dimpase commented May 29, 2025

@vbraun there are no changes to anything TeX-typesettable here.

@user202729
Copy link
Contributor

Does pdf docbuild work? I don't remember what the error was, but pretty sure the merge script failed because of that.

And yes, the point of running TeX is to verify that the equation are valid markup.

Not on the CI at the moment, but that was broken by a combination of previous pull requests. My understanding (best guess?) of the situation is explained in #40157 .

I'm not sure if build-doc-pdf depends on build-doc in some way however. A quick reading shows no dependency.

@vbraun
Copy link
Member

vbraun commented May 29, 2025

----------------------------------------------------------------------
sage -t --long --warn-long 30.0 --random-seed=123 src/sage_docbuild/builders.py  # 16 doctests failed
----------------------------------------------------------------------

@dimpase
Copy link
Member

dimpase commented May 29, 2025

I am trying to run meson compile -C builddir doc-html (after doing meson install) and I am getting errors
such as the following

[2/189] Generating src/doc/doc-inventory-reference-references with a custom command
[reference] Configuration error!
[reference] Versions
[reference] ========
[reference] * Platform:         linux; (Linux-6.13.4-gentoo-x86_64-x86_64-Intel-R-_Core-TM-_Ultra_7_165U-with-glibc2.41)
[reference] * Python version:   3.13.3 (CPython)
[reference] * Sphinx version:   8.2.3
[reference] * Docutils version: 0.21.2
[reference] * Jinja2 version:   3.1.6
[reference] * Pygments version: 2.19.1
[reference] Last Messages
[reference] =============
[reference] None.
[reference] Loaded Extensions
[reference] =================
[reference] None.
[reference] Traceback
[reference] =========
[reference]       File "/usr/lib/python3.13/site-packages/sphinx/config.py", line 616, in eval_config_file
[reference]         raise ConfigError(msg % traceback.format_exc()) from exc
[reference]     sphinx.errors.ConfigError: There is a programmable error in your configuration file:
[reference]     Traceback (most recent call last):
[reference]       File "/usr/lib/python3.13/site-packages/sphinx/config.py", line 601, in eval_config_file
[reference]         exec(code, namespace)  # NoQA: S102
[reference]         ~~~~^^^^^^^^^^^^^^^^^
[reference]       File "/home/dima/software/sage-src/src/doc/en/reference/references/conf.py", line 19, in <module>
[reference]         for tag in feature_tags():
[reference]                    ^^^^^^^^^^^^
[reference]     NameError: name 'feature_tags' is not defined
[reference] The full traceback has been saved in:

which seem to indicate that no sphinx extensions are loaded. Why?

@dimpase
Copy link
Member

dimpase commented May 29, 2025

@tobiasdiez : what is feature_tags It's something not defined in Sage sources

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants